Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

ktor-client/ktor-client-core/web/src/io/ktor/client/utils/JsUtils.web.kt (1)

54-64: Potential race when multiple events fire in the same microtask.

When addOneTimeEventListener is called with multiple event names (e.g., "open", "error"), if both events fire before dispose() finishes removing all listeners, the callback could execute twice. JavaScript's single-threaded model doesn't fully protect against this since both events could be queued and processed sequentially before the synchronous dispose() loop completes for all event names.

In practice, for the current usage (awaitConnection with "open"/"error"), a WebSocket typically fires only one of these events, so this may be acceptable. However, the KDoc should document this limitation, or consider adding a guard (e.g., an AtomicBoolean or captured flag) to ensure single invocation.

♻️ Suggested documentation or guarded implementation

Option 1: Document the limitation in KDoc

 /**
  * Registers a one-time event listener that removes itself after the first event fires.
  * The listener is registered for all specified [events] and fires once for whichever event occurs first.
+ *
+ * Note: If multiple registered events fire in quick succession before [dispose] completes,
+ * the [listener] may be invoked more than once. Callers relying on single invocation should
+ * ensure the underlying event source only fires one of the registered events.
  *
  * The event is unsafe-cast to [T] before invoking [listener]. See [addEventListener] for why.

Option 2: Add a guard for guaranteed single invocation

 internal inline fun <reified T : Event> EventTarget.addOneTimeEventListener(
     vararg events: String,
     crossinline listener: (T) -> Unit
 ): DisposableHandle {
+    var fired = false
     lateinit var disposable: DisposableHandle
     disposable = addEventListener<T>(*events) { event ->
+        if (fired) return@addEventListener
+        fired = true
         disposable.dispose()
         listener(event)
     }
     return disposable
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-client/ktor-client-core/web/src/io/ktor/client/utils/JsUtils.web.kt`
around lines 54 - 64, The addOneTimeEventListener implementation for EventTarget
can invoke the listener twice if multiple events fire before dispose removes all
listeners; update EventTarget.addOneTimeEventListener to capture a local guard
flag (e.g., var invoked = false) closed over by the returned listener so the
first invocation atomically checks and sets invoked (if already true, return),
then calls disposable.dispose() and listener(event); also add a short KDoc note
on EventTarget.addOneTimeEventListener explaining the single-invocation
guarantee. Ensure references to DisposableHandle, addEventListener and
EventTarget.addOneTimeEventListener are used when making the change.

ktor-client/ktor-client-core/wasmJs/src/io/ktor/client/plugins/websocket/JsWebSocketSession.kt (1)

97-102: Use event.message for a more informative error message in WebSocket error events.

Using "$event" produces unhelpful output like [object ErrorEvent]. The ErrorEvent type has a message property that provides actionable diagnostic information, as used in the corresponding engine error handlers.

♻️ Suggested improvement

         val onError = websocket.addEventListener<ErrorEvent>("error") { event ->
-            val cause = WebSocketException("$event")
+            val cause = WebSocketException(event.message)
             _closeReason.completeExceptionally(cause)
             _incoming.close(cause)
             _outgoing.cancel()
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-core/wasmJs/src/io/ktor/client/plugins/websocket/JsWebSocketSession.kt`
around lines 97 - 102, The error handler attached as onError constructs a
WebSocketException from "$event" which yields unhelpful "[object ErrorEvent]";
change the construction to use the ErrorEvent message (e.g., event.message,
falling back to event.toString() if empty) when creating the WebSocketException
in websocket.addEventListener, so
_closeReason.completeExceptionally(WebSocketException(...)),
_incoming.close(...), and _outgoing.cancel() carry the real diagnostic text;
update references in the onError handler where WebSocketException is created to
use event.message.

ktor-client/ktor-client-core/web/src/io/ktor/client/utils/JsUtils.web.kt (1)

57-57: Variable shadowing: consider renaming to avoid confusion.

The parameter events is shadowed by the local val events. While this works, it can be confusing during debugging or maintenance.

♻️ Suggested rename

-    val events = listOf(event, *events)
+    val allEvents = listOf(event, *events)
     lateinit var callback: (JsAny) -> Unit
-    val disposable = DisposableHandle { events.forEach { removeEventListener(it, callback) } }
+    val disposable = DisposableHandle { allEvents.forEach { removeEventListener(it, callback) } }
     callback = eventCallback<T> {
         if (once) disposable.dispose()
         listener(it)
     }
-    events.forEach { addEventListener(it, callback) }
+    allEvents.forEach { addEventListener(it, callback) }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-client/ktor-client-core/web/src/io/ktor/client/utils/JsUtils.web.kt` at
line 57, The local declaration `val events = listOf(event, *events)` shadows the
function parameter `events`; rename the local variable (e.g., `combinedEvents`
or `eventsList`) and update all subsequent references in the same function to
use that new name so the parameter and local list are not ambiguous; ensure any
spread (`*events`) still references the original parameter and the rest of the
logic (like iterating or passing the merged list) uses the renamed local
variable.

ktor-client/ktor-client-core/wasmJs/src/io/ktor/client/plugins/websocket/JsWebSocketSession.kt (1)

97-102: Consider using the eventAsString pattern for safer error event handling.

The ws npm package (used for Node.js WebSockets) may emit error events that don't have a message property like the standard DOM ErrorEvent interface. The code in WasmJsClientEngine.kt already uses eventAsString(event) to safely serialize the entire event object when handling errors during connection. Apply the same pattern here for consistency and robustness across the error handling code path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-core/wasmJs/src/io/ktor/client/plugins/websocket/JsWebSocketSession.kt`
around lines 97 - 102, The onError handler in JsWebSocketSession.kt constructs a
WebSocketException from event.message which may be missing for non-DOM
ErrorEvent instances; change it to use the same safe serialization used
elsewhere (call eventAsString(event) to produce the error text) and pass that
into WebSocketException, then continue to complete _closeReason exceptionally,
close _incoming with the cause, and cancel _outgoing as before; reference the
existing onError val, WebSocketException, _closeReason, _incoming, _outgoing,
and the eventAsString utility used in WasmJsClientEngine.kt to locate where to
update the handler.

ktor-client/ktor-client-core/web/src/io/ktor/client/utils/JsUtils.web.kt (1)

57-57: Variable shadows parameter name.

The local val events shadows the vararg events parameter. While this works correctly, consider renaming for clarity.

♻️ Suggested rename for clarity

 ): DisposableHandle {
-    val events = listOf(event, *events)
+    val allEvents = listOf(event, *events)
     lateinit var callback: (JsAny) -> Unit
-    val disposable = DisposableHandle { events.forEach { removeEventListener(it, callback) } }
+    val disposable = DisposableHandle { allEvents.forEach { removeEventListener(it, callback) } }
     callback = eventCallback<T> {
         if (once) disposable.dispose()
         listener(it)
     }
-    events.forEach { addEventListener(it, callback) }
+    allEvents.forEach { addEventListener(it, callback) }
     return disposable
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-client/ktor-client-core/web/src/io/ktor/client/utils/JsUtils.web.kt` at
line 57, The local declaration val events = listOf(event, *events) shadows the
vararg parameter named events; rename the local variable (e.g., combinedEvents
or eventsList) wherever used in the function to avoid shadowing and improve
clarity (update the declaration and all subsequent references in the function
such as usages of the newly named combinedEvents/eventsList instead of events).